-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Use newtype_index!
-generated types more idiomatically
#139811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
rustbot has assigned @matthewjasper. Use |
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
This comment has been minimized.
This comment has been minimized.
@@ -606,7 +606,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>( | |||
// with placeholders, which imply nothing about outlives bounds, and then | |||
// prove below that the hidden types are well formed. | |||
let universe = infcx.create_next_universe(); | |||
let mut idx = 0; | |||
let mut idx = ty::BoundVar::ZERO; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh fun, I didn't realize newtype indices supported AddAssign<usize>
or impls for other basic int types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it was just added in the followed-up PR in the description; but these newtypes impl'd Add<usize>
already so it makes sense to have had it in the first place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep.
I think they may also benefit from Add<Self>
and AddAssign<Self>
, but adding AddAssign<usize>
was easier to justify (since Add<usize>
was already impl'd, as @compiler-errors pointed out)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self adding doesn't make too much sense for indices imo, at least not without a good use case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't think indices should have Add<Self>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I had a good example for a use-case for Add<Self>
, but I might have imagined it, and on second thought - yeah, doesn't make a lot of sense to have that impl. Not that I added it, anyway 😄
5706da5
to
4b63362
Compare
@bors r+ rollup |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#139745 (Avoid unused clones in `Cloned<I>` and `Copied<I>`) - rust-lang#139757 (opt-dist: use executable-extension for host llvm-profdata) - rust-lang#139778 (Add test for issue 34834) - rust-lang#139783 (Use `compiletest-ignore-dir` for bootstrap self-tests) - rust-lang#139797 (Allow (but don't require) `#[unsafe(naked)]` so that `compiler-builtins` can upgrade to it) - rust-lang#139799 (Specify `--print info=file` syntax in `--help`) - rust-lang#139811 (Use `newtype_index!`-generated types more idiomatically) - rust-lang#139813 (Miri subtree update) r? `@ghost` `@rustbot` modify labels: rollup
…li-obk Use `newtype_index!`-generated types more idiomatically Continuation of sorts of rust-lang#139674 Shouldn't affect anything, just makes some code simpler
Rollup of 11 pull requests Successful merges: - rust-lang#139669 (Overhaul `AssocItem`) - rust-lang#139671 (Proc macro span API redesign: Replace proc_macro::SourceFile by Span::{file, local_file}) - rust-lang#139750 (std/thread: Use default stack size from menuconfig for NuttX) - rust-lang#139785 (Let CStrings be either 1 or 2 byte aligned.) - rust-lang#139797 (Allow (but don't require) `#[unsafe(naked)]` so that `compiler-builtins` can upgrade to it) - rust-lang#139798 (normalize: prefer `ParamEnv` over `AliasBound` candidates) - rust-lang#139799 (Specify `--print info=file` syntax in `--help`) - rust-lang#139811 (Use `newtype_index!`-generated types more idiomatically) - rust-lang#139813 (Miri subtree update) - rust-lang#139822 (Fix: Map EOPNOTSUPP to ErrorKind::Unsupported on Unix) - rust-lang#139836 (Basic tests of MPMC receiver cloning) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#139745 (Avoid unused clones in `Cloned<I>` and `Copied<I>`) - rust-lang#139757 (opt-dist: use executable-extension for host llvm-profdata) - rust-lang#139778 (Add test for issue 34834) - rust-lang#139783 (Use `compiletest-ignore-dir` for bootstrap self-tests) - rust-lang#139797 (Allow (but don't require) `#[unsafe(naked)]` so that `compiler-builtins` can upgrade to it) - rust-lang#139799 (Specify `--print info=file` syntax in `--help`) - rust-lang#139811 (Use `newtype_index!`-generated types more idiomatically) - rust-lang#139813 (Miri subtree update) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#139811 - yotamofek:pr/newtype_cleanups, r=oli-obk Use `newtype_index!`-generated types more idiomatically Continuation of sorts of rust-lang#139674 Shouldn't affect anything, just makes some code simpler
Continuation of sorts of #139674
Shouldn't affect anything, just makes some code simpler